Fix/issues#18447，And a module for setting Beta function Settings has been added, allowing users to control whether to enable certain functions #20352
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR fixes several issues related to variable validations across workflow node types and adds a new beta configuration module to allow workspace creators to control experimental features. Key changes include:
- Adding and synchronizing checkVarValid functions to validate variables in various workflow nodes.
- Integrating beta configuration both in the backend (table modifications, API endpoints) and frontend (account settings, beta page).
- Minor refactoring of import statements and adjustments to dependency updates in checklist hooks.
Reviewed Changes
Copilot reviewed 33 out of 33 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| web/app/components/workflow/nodes/* | Added checkVarValid functions for variable validation across multiple node types. |
| web/app/components/workflow/header/account-setting/* | Introduced beta configuration UI support and adjusted component imports. |
| api/* | Updated database schema and API endpoints to support beta_config. |
Comments suppressed due to low confidence (1)
web/app/components/header/account-setting/index.tsx:36
- Consider renaming the imported component from 'BatePage' to 'BetaPage' to maintain consistency with conventional naming.
import BatePage from './beta-page'
| checkVarValid(payload: AssignerNodeType, varMap: Record<string, Var>, t: any) { | ||
| const errorMessageArr: string[] = [] | ||
| const items = payload.items ?? [] | ||
| const variables_warnings = getNotExistVariablesByArray(items.map(item => item.variable_selector ?? []) ?? [], varMap) |
There was a problem hiding this comment.
Ensure that 'item.variable_selector' consistently returns a string value; using an empty array as a fallback may lead to type inconsistencies in variable validation.
Copilot uses AI. Check for mistakes.
| if (variables_warnings.length) | ||
| errorMessageArr.push(`${t('workflow.nodes.assigner.assignedVariable')} ${t('workflow.common.referenceVar')}${variables_warnings.join('、')}${t('workflow.common.noExist')}`) | ||
|
|
||
| const value_warnings = getNotExistVariablesByArray(items.map(item => item.value ?? []) ?? [], varMap) |
There was a problem hiding this comment.
Ensure that 'item.value' consistently returns a string value; using an empty array as a fallback may lead to unexpected behavior during variable validation.
Copilot uses AI. Check for mistakes.
| @@ -199,6 +199,7 @@ class Tenant(Base): | |||
| plan = db.Column(db.String(255), nullable=False, server_default=db.text("'basic'::character varying")) | |||
| status = db.Column(db.String(255), nullable=False, server_default=db.text("'normal'::character varying")) | |||
| custom_config = db.Column(db.Text) | |||
| beta_config = db.Column(db.Text) | |||
There was a problem hiding this comment.
I have reviewed the issue you mentioned, but I'm having trouble understanding why we need this field. It appears to store an unvalidated JSON structure directly in the database. Could you please provide a detailed explanation of its purpose?
There was a problem hiding this comment.
I need to set a switch or configuration for the Beta function of the workspace, so I need this field. But what you said is an unverified JSON structure, which does seem to be an unsafe design indeed
|
There are still some issues in this PR that failed the test |
|
@tinet-dongfb I tested this feature, and it effectively detects non-existent variables. I believe we can implement it as a permanent feature without requiring a beta toggle. |
Summary
Fixes #18447
The previously submitted PR caused some issues, so this time it's not just about fixing the problems that emerged；
Fixes #19784
Fixes #19782
Fixes #19771
Fixes #19768
PR: #19732， PR: #19744 The modified code has been modified synchronously
In addition, a configuration page for the Beta function has been added. It is hoped that the creator of the workspace can decide for themselves whether to enable this function
But I 'm not a pure backend developer, although beta_config was added to the tenants table, do I know how to submit changes to the table structure to you? Hope you can handle the structural change of this table. The table structure statement is
Some front-end unit tests will be added for this function in the future, but this will take a little time
Screenshots
Checklist
Important
Please review the checklist below before submitting your pull request.
dev/reformat(backend) andcd web && npx lint-staged(frontend) to appease the lint gods